-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Properly handle <br> and \n in scattermapbox text #4176
Conversation
... to be consistent with the rest of plotly.js
... to show #4152 fix
@@ -20,6 +20,9 @@ var makeBubbleSizeFn = require('../scatter/make_bubble_size_func'); | |||
var subTypes = require('../scatter/subtypes'); | |||
var convertTextOpts = require('../../plots/mapbox/convert_text_opts'); | |||
|
|||
var NEWLINES = require('../../lib/svg_text_utils').NEWLINES; | |||
var BR_TAG_ALL = require('../../lib/svg_text_utils').BR_TAG_ALL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking: @etpinard would you mind requiring svg_text_utils
once?
@@ -261,6 +263,9 @@ function makeSymbolGeoJSON(calcTrace, gd) { | |||
calcPt.txt = Lib.texttemplateString(txti, {}, gd._fullLayout._d3locale, calcPt, trace._meta || {}); | |||
} | |||
|
|||
var text = txt ? calcPt.txt : fillText(calcPt.tx); | |||
if(text) text = text.replace(NEWLINES, '').replace(BR_TAG_ALL, '\n'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting.
Wondering this logic may be useful in other places.
If so, it would be nice to have it in Lib
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's part of
plotly.js/src/lib/svg_text_utils.js
Lines 449 to 626 in 4ef2f80
function buildSVGText(containerNode, str) { | |
/* | |
* Normalize behavior between IE and others wrt newlines and whitespace:pre | |
* this combination makes IE barf https://github.com/plotly/plotly.js/issues/746 | |
* Chrome and FF display \n, \r, or \r\n as a space in this mode. | |
* I feel like at some point we turned these into <br> but currently we don't so | |
* I'm just going to cement what we do now in Chrome and FF | |
*/ | |
str = str.replace(NEWLINES, ' '); | |
var hasLink = false; | |
// as we're building the text, keep track of what elements we're nested inside | |
// nodeStack will be an array of {node, type, style, href, target, popup} | |
// where only type: 'a' gets the last 3 and node is only added when it's created | |
var nodeStack = []; | |
var currentNode; | |
var currentLine = -1; | |
function newLine() { | |
currentLine++; | |
var lineNode = document.createElementNS(xmlnsNamespaces.svg, 'tspan'); | |
d3.select(lineNode).attr({ | |
class: 'line', | |
dy: (currentLine * LINE_SPACING) + 'em' | |
}); | |
containerNode.appendChild(lineNode); | |
currentNode = lineNode; | |
var oldNodeStack = nodeStack; | |
nodeStack = [{node: lineNode}]; | |
if(oldNodeStack.length > 1) { | |
for(var i = 1; i < oldNodeStack.length; i++) { | |
enterNode(oldNodeStack[i]); | |
} | |
} | |
} | |
function enterNode(nodeSpec) { | |
var type = nodeSpec.type; | |
var nodeAttrs = {}; | |
var nodeType; | |
if(type === 'a') { | |
nodeType = 'a'; | |
var target = nodeSpec.target; | |
var href = nodeSpec.href; | |
var popup = nodeSpec.popup; | |
if(href) { | |
nodeAttrs = { | |
'xlink:xlink:show': (target === '_blank' || target.charAt(0) !== '_') ? 'new' : 'replace', | |
target: target, | |
'xlink:xlink:href': href | |
}; | |
if(popup) { | |
// security: href and target are not inserted as code but | |
// as attributes. popup is, but limited to /[A-Za-z0-9_=,]/ | |
nodeAttrs.onclick = 'window.open(this.href.baseVal,this.target.baseVal,"' + | |
popup + '");return false;'; | |
} | |
} | |
} else nodeType = 'tspan'; | |
if(nodeSpec.style) nodeAttrs.style = nodeSpec.style; | |
var newNode = document.createElementNS(xmlnsNamespaces.svg, nodeType); | |
if(type === 'sup' || type === 'sub') { | |
addTextNode(currentNode, ZERO_WIDTH_SPACE); | |
currentNode.appendChild(newNode); | |
var resetter = document.createElementNS(xmlnsNamespaces.svg, 'tspan'); | |
addTextNode(resetter, ZERO_WIDTH_SPACE); | |
d3.select(resetter).attr('dy', RESET_DY[type]); | |
nodeAttrs.dy = SHIFT_DY[type]; | |
currentNode.appendChild(newNode); | |
currentNode.appendChild(resetter); | |
} else { | |
currentNode.appendChild(newNode); | |
} | |
d3.select(newNode).attr(nodeAttrs); | |
currentNode = nodeSpec.node = newNode; | |
nodeStack.push(nodeSpec); | |
} | |
function addTextNode(node, text) { | |
node.appendChild(document.createTextNode(text)); | |
} | |
function exitNode(type) { | |
// A bare closing tag can't close the root node. If we encounter this it | |
// means there's an extra closing tag that can just be ignored: | |
if(nodeStack.length === 1) { | |
Lib.log('Ignoring unexpected end tag </' + type + '>.', str); | |
return; | |
} | |
var innerNode = nodeStack.pop(); | |
if(type !== innerNode.type) { | |
Lib.log('Start tag <' + innerNode.type + '> doesnt match end tag <' + | |
type + '>. Pretending it did match.', str); | |
} | |
currentNode = nodeStack[nodeStack.length - 1].node; | |
} | |
var hasLines = BR_TAG.test(str); | |
if(hasLines) newLine(); | |
else { | |
currentNode = containerNode; | |
nodeStack = [{node: containerNode}]; | |
} | |
var parts = str.split(SPLIT_TAGS); | |
for(var i = 0; i < parts.length; i++) { | |
var parti = parts[i]; | |
var match = parti.match(ONE_TAG); | |
var tagType = match && match[2].toLowerCase(); | |
var tagStyle = TAG_STYLES[tagType]; | |
if(tagType === 'br') { | |
newLine(); | |
} else if(tagStyle === undefined) { | |
addTextNode(currentNode, convertEntities(parti)); | |
} else { | |
// tag - open or close | |
if(match[1]) { | |
exitNode(tagType); | |
} else { | |
var extra = match[4]; | |
var nodeSpec = {type: tagType}; | |
// now add style, from both the tag name and any extra css | |
// Most of the svg css that users will care about is just like html, | |
// but font color is different (uses fill). Let our users ignore this. | |
var css = getQuotedMatch(extra, STYLEMATCH); | |
if(css) { | |
css = css.replace(COLORMATCH, '$1 fill:'); | |
if(tagStyle) css += ';' + tagStyle; | |
} else if(tagStyle) css = tagStyle; | |
if(css) nodeSpec.style = css; | |
if(tagType === 'a') { | |
hasLink = true; | |
var href = getQuotedMatch(extra, HREFMATCH); | |
if(href) { | |
// check safe protocols | |
var dummyAnchor = document.createElement('a'); | |
dummyAnchor.href = href; | |
if(PROTOCOLS.indexOf(dummyAnchor.protocol) !== -1) { | |
// Decode href to allow both already encoded and not encoded | |
// URIs. Without decoding prior encoding, an already encoded | |
// URI would be encoded twice producing a semantically different URI. | |
nodeSpec.href = encodeURI(decodeURI(href)); | |
nodeSpec.target = getQuotedMatch(extra, TARGETMATCH) || '_blank'; | |
nodeSpec.popup = getQuotedMatch(extra, POPUPMATCH); | |
} | |
} | |
} | |
enterNode(nodeSpec); | |
} | |
} | |
} | |
return hasLink; | |
} |
used for all SVG text.
@etpinard nicely done. |
fixes #4152
In mapbox-gl, users have to input
\n
to break lines in text while we expect<br>
to do that. This PR fixes the inconsistencies betweenscattermapbox
text and all other plotly.js text attributes.before: https://codepen.io/etpinard/pen/vYBdoeJ
after: https://codepen.io/etpinard/pen/OJLQKxZ
cc @plotly/plotly_js